-
Notifications
You must be signed in to change notification settings - Fork 336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SES lockdown to testrunners #605
Conversation
@willclarktech do you get this error locally as well (see CI)?
Is it because |
@webmaster128 Yes, I missed this locally because I was running the tests with Node v12. |
@kriskowal what Node.js versions are supported by the "ses" package? Is this documented somewhere? Node.js 10 will EOL at the end of April, so we could think about dropping it. But it would be good to get this documented. |
@willclarktech what about a separate SES CI job that runs the tests in node.js 12 and we have something like if (provess.env.SES_ENABLED) {
require("ses/lockdown");
// eslint-disable-next-line no-undef
lockdown();
} |
ec73cfc
to
a2076ef
Compare
a2076ef
to
6c9b5ae
Compare
Except @cosmjs/cli
0741a46
to
9f45346
Compare
We at least informally support Node.js 10 since our partners at MetaMask use that too. cc @kumavis. For those cases, SES ships with a rolled-up CommonJS distribution like |
Hey, what's the status of this? Should be part of our service agreement with Agoric. |
This is done and running. The remaining trouble was resolved by dropping support for the now unmaintained Node.js 10. Does this answer your question @hxrts? I don't know about this service agreement. |
Ah, should have read above. Yes it does, though @kriskowal can maybe comment on whether Agoric plans to do anything further. |
The best person to discuss the scope of Agoric’s service agreement is Vanessa Pestritto at Agoric. My expectation is that I am to be available to assist if ever a problem arises with compatibility of a new dependency and to provide help upgrading SES, particularly for breaking-changes that might impact compatibility with CosmJS dependencies. Notably, we’re planning a version bump (0.+1) to tighten security by forbidding the use of the Node.js |
Part of #412
The CLI package still has the obscure error, so I couldn’t add SES to the testrunner there. Do we
want SES added to our applications? (Eg adding it now to the faucet.)